-
Notifications
You must be signed in to change notification settings - Fork 473
Fix memory leak by changing now playing artwork request handler #3051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix memory leak by changing now playing artwork request handler #3051
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a memory leak in the MediaElement component on iOS/macOS by refactoring how the now playing artwork request handler is managed. The memory leak occurred due to a reference cycle where MPMediaItemArtwork captured the mediaElement instance through its request handler delegate, while the Metadata class holding the MPMediaItemArtwork was itself referenced by the media element.
Changes:
- Introduced a new
RequestHandlerProxyclass that captures only the artwork URL string instead of the entire media element object - Added
CoreGraphicsusing directive for theCGSizetype - Refactored the artwork request handler to use the proxy class method reference instead of a lambda
src/CommunityToolkit.Maui.MediaElement/Primitives/Metadata.macios.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
src/CommunityToolkit.Maui.MediaElement/Primitives/Metadata.macios.cs
Outdated
Show resolved
Hide resolved
ne0rrmatrix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ty for the PR. Good catch. I have tested this in the simulator and I see no issues with this.
Description of Change
Changes the request handler for the now playing artwork to only capture the actual url string. This breaks a reference cycle with
MPNowPlayingInfoandMPMediaItemArtworkwhereMPMediaItemArtworkkeeps itself alive through the request handler delegate capturing the whole media element which holds their handler, holding theMetadataclass, holdingMPNowPlayingInfo, which finally holdsMPMediaItemArtworkagain. I did not add a test, since the changes are minimal, just changing how the artwork url is captured to avid the reference cycle.Linked Issues
PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information